Add Navigation Overlay close block (behind experiment)#73724
Conversation
getdave
changed the title
|
@mikachan as you've been working a lot on new blocks lately would you be able to take a look at the implementation here to see it follows best practices? 🙏 |
|
@mcsf @ryanwelcher I'm aware that there is a proposal for an Icons API/Registry and also potentially a Icon block. How do you see the block in this PR working with that system? Note that we're going to need to add some pretty specific interactivity to this block when rendering within the Navigation block on the front of the site (toggle Nav block overlay open/close...etc) so I do think it needs to be its own block. However, I'm keen to see how we could allow for things like customising the Icon...etc. I'd very much appreciate your input here. |
|
@jasmussen If we go ahead with this block we're going to need a custom icon 🙏 I don't think we should use |
getdave
added
New Block
|
Size Change: +1.96 kB (+0.08%) Total Size: 2.58 MB
ℹ️ View Unchanged
|
|
A thought: 
SVG in case @WordPress/gutenberg-design agrees: |
I trust you on the grounds for having a separate block for this. As for the rest:
|
|
Appreciate the input here @mcsf
Correct 👍
Yeh this is what I was getting at. I want to set up the block so that it's simple/trivial to transition to the Icon Registry in the future. I'll go with what you suggested.
Yeh I thought that could be the case. I'll keep an eye on the efforts in case they provide additional functionality for this block pre-release. |
| @@ -0,0 +1,57 @@ | |||
| { | |||
| "$schema": "https://schemas.wp.org/trunk/block.json", | |||
There was a problem hiding this comment.
Should this have a "parent" definition to restrict insertion to the overlay template part?
There was a problem hiding this comment.
Interesting. I didn't think that would work at the entity level. I'll give that a try now.
There was a problem hiding this comment.
It doesn't. I've used a filter to restrict insertion to when we're editing an overlay template part.
It still shows up in the inserter but that's because we don't seem to have a way to filter that dynamically as it relies on getBlockType settings.
We could consider hiding blocks that can't be inserted from the inserter. That would seem to make sense.
There was a problem hiding this comment.
While we can't use "parent" since the Navigation Overlay isn't a distinct block type (but rather a core/template-part with area set to overlay), we could still use it to limit the usage of the block to Template Parts (in addition to the filter that Dave added). This would at least prevent the block from showing up outside of Template Parts.
Aside, maybe "ancestor" instead of "parent" would be a better fit? In case the Overlay Close button isn't a direct child of a Navigation Overlay but e.g. inside of some Group or Column block 🤔
diff --git a/packages/block-library/src/navigation-overlay-close/block.json b/packages/block-library/src/navigation-overlay-close/block.json
index 96cdabca2a2..aade2aaf570 100644
--- a/packages/block-library/src/navigation-overlay-close/block.json
+++ b/packages/block-library/src/navigation-overlay-close/block.json
@@ -6,6 +6,7 @@
"category": "design",
"description": "A customizable button to close overlays.",
"keywords": [ "close", "overlay", "navigation", "menu" ],
+ "ancestor": [ "core/template-part" ],
"textdomain": "default",
"attributes": {
"displayMode": {| Before | After |
|---|---|
 |
 |
There was a problem hiding this comment.
Turns out this won't work because when in focused template part editing mode we don't have a template part block. As a result with that attribute in place you can no longer add a Close block to the overlay template.
There was a problem hiding this comment.
Turns out this won't work because when in focused template part editing mode we don't have a template part block. As a result with that attribute in place you can no longer add a Close block to the overlay template.
You're right 😢
BTW this raises the question if we should make Gutenberg smart enough to understand that if we're currently in focused "x entity" editing mode, and a block includes x in its "parent" or "ancestor" fields, then it should be included in the inserter. I might look into that.
There was a problem hiding this comment.
I think it should yes. Or we allow blocks to define the inserter setting as a function. That function would need sufficient context to be able to determine whether or not the block should show in a given scenario.
There was a problem hiding this comment.
I've filed an issue: #73827
Or we allow blocks to define the
insertersetting as a function. That function would need sufficient context to be able to determine whether or not the block should show in a given scenario.
One thing I've discussed with @mtias is to
- allow block variations in places such as the
"parent"and"ancestor"fields inblock.json(using a syntax where the variation name is slash-appended to the block name, e.g.core/heading/h2), and to - create
core/template-partvariations for eacharea(resulting in e.g.core/template-part/overlay).
Together, those changes should allow us to limit the block to the Navigation Overlay.
(Note that @mtias is also modifying the behavior of the block scope in variations: #73823)
There was a problem hiding this comment.
- allow block variations in places such as the
"parent"and"ancestor"fields inblock.json(using a syntax where the variation name is slash-appended to the block name, e.g.core/heading/h2)
Draft PR: #73828
There was a problem hiding this comment.
@ockham Thank you for your work here. As it's experimental, I'm going to go ahead and merge the block and then follow up to include your work when it lands as it looks to be exactly what we'll need here 🙇
getdave
requested review from
ajitbohra,
fabiankaegy and
juanmaguitar
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
mikachan
left a comment
mikachan
left a comment
There was a problem hiding this comment.
This block looks great so far, looking forward to bringing it in and seeing how it'll work on the front end.
This block currently uses a hardcoded close icon (X). In the future, we may want to support the proposed Icons API and/or an Icon block to allow users to customize the icon. We should carefully consider how to implement this while maintaining backwards compatibility with existing blocks that use the current hardcoded icon approach.
Another option for the icon here could be to use a + character, rotated 45 degrees. This option allows for slightly more flexibility for styling and animations. This is how we're handling the close icon in the Accordion Heading (although it's initially not rotated, and then rotated 45 degrees when expanded). However, I think for this block it's best to stick with the hardcoded close icon for now. I'm mainly just throwing another idea out there from another block that's planning to use the Icons API in the future.
|
Flaky tests detected in 48ab8c2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/20129333476
|
getdave
changed the title
getdave
force-pushed
the
add-overlay-close-block
branch
from
49291b0 to
abccb9e
Compare
getdave
force-pushed
the
add-overlay-close-block
branch
from
f9ab9fa to
e2db952
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a new experimental core/navigation-overlay-close block that provides a customizable close button for navigation overlays, implementing a block-based approach that allows users to style the close button using standard block editing tools instead of requiring custom controls on the Navigation block.
Key changes:
- Introduces the
core/navigation-overlay-closeblock with three display modes (icon, text, or both) and full support for color, spacing, and typography customization - Restricts block insertion to overlay template parts only through a filter on
blockEditor.__unstableCanInsertBlockType - Gates the feature behind the
gutenberg-customizable-navigation-overlaysexperiment flag
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/block-library/src/navigation-overlay-close/block.json | Defines block metadata, attributes (displayMode, text), and supports for color, spacing, and typography |
| packages/block-library/src/navigation-overlay-close/edit.js | Editor component with display mode controls and RichText editing for button text |
| packages/block-library/src/navigation-overlay-close/save.js | Renders button element with conditional icon/text display based on displayMode attribute |
| packages/block-library/src/navigation-overlay-close/index.js | Block registration and filter logic to restrict insertion to overlay template parts |
| packages/block-library/src/navigation-overlay-close/icon.js | Block icon definition using SVG primitives |
| packages/block-library/src/navigation-overlay-close/index.php | Server-side registration function for the block |
| packages/block-library/src/navigation-overlay-close/style.scss | Frontend styles for button appearance, hover, and focus states |
| packages/block-library/src/navigation-overlay-close/editor.scss | Editor-specific styles for minimum button dimensions |
| packages/block-library/src/index.js | Conditionally registers the block when __experimentalNavigationOverlays flag is enabled |
| docs/reference-guides/core-blocks.md | Adds documentation entry for the new Navigation Overlay Close block |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| - **Name:** core/navigation-overlay-close | ||
| - **Category:** design | ||
| - **Supports:** color (background, text, ~~gradients~~), spacing (padding), typography (fontSize, lineHeight) |
There was a problem hiding this comment.
The typography supports documentation is incomplete. The block.json file shows that the block supports additional typography features including fontFamily, fontWeight, fontStyle, textTransform, textDecoration, and letterSpacing, but the documentation only lists fontSize and lineHeight.
| - **Supports:** color (background, text, ~~gradients~~), spacing (padding), typography (fontSize, lineHeight) | |
| - **Supports:** color (background, text, ~~gradients~~), spacing (padding), typography (fontSize, lineHeight, fontFamily, fontWeight, fontStyle, textTransform, textDecoration, letterSpacing) |
|
|
||
| ## Navigation Overlay Close | ||
|
|
||
| A customizable button to close overlays. ([Source](https://github.com/WordPress/gutenberg/tree/trunk/packages/block-library/src/navigation-overlay-close)) |
There was a problem hiding this comment.
Maybe add a note about when it's allowed to be inserted (overlay template part)
There was a problem hiding this comment.
That would require updating the block description in block.json as this is auto generated from that. Not sure 😬
| min-height: 44px; | ||
| min-width: 44px; |
There was a problem hiding this comment.
Do we have some scss variables we could use instead? Maybe a min button size variable?
There was a problem hiding this comment.
Also, could the font-size impact this icon? Right now the icon is always the same size even when interacting with the font size scale. It feels a bit odd to have the close text really large and the icon stay small. I'm not sure using font size is the right approach for customizing that, but it might work well.
There was a problem hiding this comment.
I've switched to em so it's proportional.
I think we should look at a control for icon size in a followup though. It's likely to be needed.
jeryj
left a comment
jeryj
left a comment
There was a problem hiding this comment.
Since this is an experimental block, I'll approve assuming the aria-labels are fixed (should be quick, I can get them tomorrow if needed) and you look into if the experimental key is needed to properly gate the block.
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
|
Thanks for all the review detail everyone. Ideally I'd like to merge this base implementation and then iterate as we go driven by requirements. |
mikachan
left a comment
mikachan
left a comment
There was a problem hiding this comment.
I agree, this looks good to bring in! 🚢
|
As the block is experimental, any further changes can be addressed in followups. |
What
Adds a new experimental
core/overlay-closeblock that provides a customizable close button for navigation overlays. The block supports three display modes (icon only, text only, or both), RichText editing with bold/italic formatting, and full block supports for color, spacing, and typography.This implements the block-based approach for the close button mentioned in #73079, allowing styling to rely on standard block affordances rather than custom controls on the Navigation block.
Why
As outlined in #73079, overlay design should prioritize stylistic changes being made when editing the overlay template itself. By providing a dedicated block for the close button, users can fully customize its appearance using standard block editing tools (color, spacing, typography) without requiring custom controls on the Navigation block.
How
core/overlay-closeblock with:@wordpress/componentsButtoncomponent in editor for consistent UIgutenberg-customizable-navigation-overlaysexperiment is enabledNotes
Future Considerations
This block currently uses a hardcoded close icon (X). In the future, we may want to support the proposed Icons API and/or an Icon block to allow users to customize the icon. We should carefully consider how to implement this while maintaining backwards compatibility with existing blocks that use the current hardcoded icon approach. This may involve extending the
displayModeattribute or introducing a newiconattribute that can accept either a predefined icon identifier or a block reference.Front end interactivity
Front-end rendering of the block with Interactivity API is not yet implemented. This will be added in future iterations.
Testing Instructions
gutenberg-customizable-navigation-overlaysexperiment in Gutenberg settingscore/overlay-closeblock to the overlay template partPart of #73079